Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

deps: cherry-pick 5005faed5 from V8 upstream #15177

Closed
wants to merge 1 commit into from
Closed

deps: cherry-pick 5005faed5 from V8 upstream #15177

wants to merge 1 commit into from

Conversation

kurayama
Copy link
Contributor

@kurayama kurayama commented Sep 4, 2017 •
edited
Loading

Original commit message:

[turbofan] Improve representation selection for type guard.

This takes into account the type of the type guard when choosing
representation for a node. To make the representation changes
unambiguous, we pass the restricted type to the changer.

BUG=chromium:726554

Review-Url: https://codereview.chromium.org/2920193004
Cr-Commit-Position: refs/heads/master@{#45734}

@nodejs-github-bot nodejs-github-bot added the v8 engine Issues and PRs related to the V8 dependency. label Sep 4, 2017
@kurayama
Copy link
Contributor Author

kurayama commented Sep 4, 2017

This cherry-pick fixes a bug that crashed our test suite with node>=8.3 when using the default v8 options (it only works with no-turbo)

@benjamingr
Copy link
Member

Thanks for making a contribution.

Is there any reason you didn't take the actual commit from V8? I think it's preferable to land the actual commit that landed in V8 so that when updating the version the commit will already be there.

Also pinging @nodejs/v8

@targos
Copy link
Member

targos commented Sep 4, 2017

@benjamingr What do you mean? This commit contains the exact same changes as the V8 commit. Anyway it doesn't matter for V8 updates because we just replace the deps/v8 directory with the new version.

@targos
Copy link
Member

targos commented Sep 4, 2017

@kurayama Thanks, this LGTM. Could you add the original commit message to your commit message too (with 4 spaces indent)? That would be perfect.

@kurayama kurayama force-pushed the fix-representation-selection branch from bc8c9c9 to dc65298 Compare September 4, 2017 12:50
deps: cherry-pick 5005faed5 from V8 upstream
40fc9ee 
Original commit message:

    [turbofan] Improve representation selection for type guard.

    This takes into account the type of the type guard when choosing
    representation for a node. To make the representation changes
    unambiguous, we pass the restricted type to the changer.

    BUG=chromium:726554

    Review-Url: https://codereview.chromium.org/2920193004
    Cr-Commit-Position: refs/heads/master@{nodejs#45734}
@kurayama kurayama force-pushed the fix-representation-selection branch from dc65298 to 40fc9ee Compare September 4, 2017 12:51
@kurayama
Copy link
Contributor Author

kurayama commented Sep 4, 2017

@targos done

jasnell
jasnell approved these changes Sep 5, 2017
View reviewed changes
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubber-stampy LGTM

@jasnell
Copy link
Member

jasnell commented Sep 7, 2017

CI: https://ci.nodejs.org/job/node-test-pull-request/9972/

jasnell pushed a commit that referenced this pull request Sep 7, 2017
@jasnell
deps: cherry-pick 5005faed5 from V8 upstream
81b2a89 
Original commit message:

    [turbofan] Improve representation selection for type guard.

    This takes into account the type of the type guard when choosing
    representation for a node. To make the representation changes
    unambiguous, we pass the restricted type to the changer.

    BUG=chromium:726554

    Review-Url: https://codereview.chromium.org/2920193004
    Cr-Commit-Position: refs/heads/master@{#45734}

PR-URL: #15177
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@jasnell
Copy link
Member

jasnell commented Sep 7, 2017

Landed in 81b2a89

@jasnell jasnell closed this Sep 7, 2017
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
@MylesBorins
deps: cherry-pick 5005faed5 from V8 upstream
c916c1c 
Original commit message:

    [turbofan] Improve representation selection for type guard.

    This takes into account the type of the type guard when choosing
    representation for a node. To make the representation changes
    unambiguous, we pass the restricted type to the changer.

    BUG=chromium:726554

    Review-Url: https://codereview.chromium.org/2920193004
    Cr-Commit-Position: refs/heads/master@{#45734}

PR-URL: #15177
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
Merged
MylesBorins pushed a commit that referenced this pull request Sep 11, 2017
@MylesBorins
deps: cherry-pick 5005faed5 from V8 upstream
15c5261 
Original commit message:

    [turbofan] Improve representation selection for type guard.

    This takes into account the type of the type guard when choosing
    representation for a node. To make the representation changes
    unambiguous, we pass the restricted type to the changer.

    BUG=chromium:726554

    Review-Url: https://codereview.chromium.org/2920193004
    Cr-Commit-Position: refs/heads/master@{#45734}

PR-URL: #15177
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
@MylesBorins
deps: cherry-pick 5005faed5 from V8 upstream
ba5a697 
Original commit message:

    [turbofan] Improve representation selection for type guard.

    This takes into account the type of the type guard when choosing
    representation for a node. To make the representation changes
    unambiguous, we pass the restricted type to the changer.

    BUG=chromium:726554

    Review-Url: https://codereview.chromium.org/2920193004
    Cr-Commit-Position: refs/heads/master@{#45734}

PR-URL: #15177
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
addaleax pushed a commit to addaleax/node that referenced this pull request Sep 13, 2017
@addaleax
deps: cherry-pick 5005faed5 from V8 upstream
410a372 
Original commit message:

    [turbofan] Improve representation selection for type guard.

    This takes into account the type of the type guard when choosing
    representation for a node. To make the representation changes
    unambiguous, we pass the restricted type to the changer.

    BUG=chromium:726554

    Review-Url: https://codereview.chromium.org/2920193004
    Cr-Commit-Position: refs/heads/master@{nodejs#45734}

PR-URL: nodejs#15177
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@MylesBorins
Copy link
Contributor

should this backport to v6.x?

@kurayama
Copy link
Contributor Author

@MylesBorins This only applies to turbofan which I believe is not present in node 6 (v8 5.1)

@kurayama kurayama deleted the fix-representation-selection branch February 18, 2019 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kurayama @benjamingr @targos @jasnell @MylesBorins @nodejs-github-bot